Add SavedModelBundle API#90
Conversation
|
I originally left out proto support because TensorFlow uses proto3 and no Rust libraries at the time supported proto3. We'll have to decide at some point whether to leave serialization/deserialization to the user, or to pick from https://github.com/stepancheg/rust-protobuf or something else. If we choose a proto library, we should be able to write the API in a way that hides the choice of library from the user, but it's still a commitment. @jhseu, do you have any thoughts on TensorFlow proto concerns? I see that the Go and Java bindings expose the config proto as a raw byte array to the user. I'd prefer any public APIs to use full-fledged structs rather than tuples or tuple structs. They have slightly more boilerplate, but they're also more extensible and explicit. Within the implementation, I think tuples and tuple structs are fine. As for the method name, to keep more consistent with the Go API and the Java API, I think it would make sense to move it to |
That is now done in 43f6bf9. |
|
Yeah, leaving serialization/deserialization up to the user seems like the most reasonable option given the fragmented protobuf situation in Rust. If there were a hard dependency, prost seems like the most reasonable protobuf crate right now, but it's too new to depend on. |
I can't speak for |
| let mut status = Status::new(); | ||
|
|
||
| let export_dir_cstr = | ||
| try!(export_dir.as_ref() |
There was a problem hiding this comment.
For new code, ? is preferred over try!.
| pub struct SavedModelBundle { | ||
| /// The loaded session. | ||
| pub session: Session, | ||
| /// A meta graph defition as raw protocol buffer. |
Is it ok for me to add a "test-resources" folder with a known saved model for this? |
|
Sure, go ahead. |
- Give explicit name to an operation in regression_savedmodel script - Add test_resources with a saved model - Add test for saved model bundle - Fix typo - Wipe try! macros
|
The requested changes were made. 🙂 |
* tensorflow#89 Remove fold/sheet limits so a full dataset can be output, albeit still limited by DisplayMinWidth/column concatenation at header width * Update Readme with arguments
3 participants
This is more of a request for feedback rather than something ready to merge now. As this concern was exposed in #68, I built an API that gathers the loaded session and meta-graph information into a bundle. When combined with the latest
protobufcrate and protoc-generated sources, it is then possible to retrieve signature definitions from the meta-graph with something like this:Since this crate is still independent from
protobuf, this extra constructor method currently provides a vector of raw bytes.Feedback would be certainly welcome. In particular, is the method's name ok? Should
SavedModelBundlebe just a tuple? Or maybe a tuple struct? This API might also depend on whether this crate will have its own protobuf serialization/deserialization code in the future. It seems that we cannot obtain certain features already available in the Python API without this.